Fix silently incorrect scores in parea/evals (two inverted metrics, eval() on model output) - #1133
Open
hassaanch23 wants to merge 6 commits into
Open
Conversation
The verdict counter was named yes_count but returned 0 for a "yes" verdict
and 1 otherwise, so it counted unsupported statements:
yes_count = sum(0 if "yes" in answer else 1 for answer in ...)
An answer fully grounded in the context scored 0.0 and a fully hallucinated
one scored 1.0 -- exactly backwards, with no error raised.
The fallback taken when the grader omits the "Final verdict for each
statement in order:" summary was wrong twice over: it counted "verdict: no"
occurrences in `output`, the answer being graded, rather than in the
grader's response, and it counted rejections as if they were supported
statements. It now counts the per-statement verdicts in the grader's reply.
The result is also clamped to 1.0, since a grader that emits more verdicts
than there were statements could otherwise push the score above 1.
context_ranking called ndcg(reranked_indices, list(range(len(contexts)))), passing a permutation of context indices where ndcg() expects relevance grades. The IDCG denominator therefore rewarded the index list being in descending order, i.e. the reranker completely reversing the retrieved order. Perfect retrieval scored 0.55 and worst-case retrieval scored 1.0 (n=4); across all permutations the score correlated -0.75 with the correct NDCG computed by sklearn. reranked_indices is now converted into relevance grades first: the context the reranker ranked highest gets the largest grade, and NDCG then measures how well the retrieved order 0..n-1 agrees with those grades. Three related fixes in the same path: - listwise_reranking parsed the model's reply with `num.isdigit()` after stripping brackets. The prompt labels passages "Passage1".."PassageN", so a reply naming them parsed to an empty list, which silently deleted the window from `contexts`, and a reply of bare numbers was 1-based but used as 0-based indices. Every integer in the reply is now pulled out, shifted to 0-based, bounds-checked and de-duplicated, and any passage the model dropped is appended, so the result is always a permutation. - progressive_reranking computed window_step = n_contexts_to_rank // 2, which is 0 for n_contexts_to_rank=1 and looped forever even though the factory explicitly accepts that value. - dcg() built gains as 2**rel on an int64 array, so a relevance grade above 62 wrapped around to a negative gain. That is reachable now that grades scale with the number of retrieved contexts. ndcg() also returned NaN instead of 0.0 when no context was relevant.
percent_target_supported_by_context ran eval() on the grader's raw reply, so
anything the model emitted was executed as Python. A reply containing
`__import__("pathlib").Path("...").write_text(...)` alongside the expected
list wrote the file before the score was computed.
The regex guard did not prevent this: it was applied to
`classification.replace("\n", "")` while eval() received the unmodified
string, so the two never saw the same text. That mismatch also broke the
ordinary case of a grader that pretty-prints its JSON or wraps it in a code
fence, which raised SyntaxError out of eval().
The matched JSON is now parsed with json.loads (via safe_json_loads) with
re.DOTALL so multi-line replies match, and a missing "Attributed" key no
longer raises AttributeError on None.
Unparseable replies now return None rather than 0.0. A hard 0.0 was
indistinguishable from "the context supports none of the target" and was
recorded as a real score; returning None skips the eval instead.
A verification response that was not valid JSON, or that omitted the "verdict" key, was mapped to np.nan. That NaN flowed into both the numerator and the denominator of the average precision, so a single malformed response out of any number silently made the whole score NaN, which was then logged as the result and destroyed any aggregate computed over the experiment. Such a response is now treated as "not relevant", matching how RAGAS handles an unusable verification. numpy is no longer needed, so the import guard and its docstring entry are dropped.
correct[log.target] += int(eval_result.score) truncates toward zero, so a class whose scores were all 0.9 was reported as having 0.0 recall. Scores are counted as correct at >= 0.5 instead.
Covers each bug fixed in this branch, driving the eval functions with scripted grader responses instead of live API calls. 22 of the 39 tests fail against the previous implementations. The ranking tests assert the property the inversion violated: over every permutation of four contexts, the score is uniquely maximised when the reranker agrees with the retrieved order and minimised when it reverses it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Five eval functions in
parea/evals/return wrong scores without raising. Two of them are inverted — they report a perfect result as a failure and a total failure as perfect. Because these run inside_make_evaluations, which swallows exceptions and logs whatever number comes back, nothing surfaces the problem: the experiment completes and the dashboard shows a plausible score that is simply wrong.Each fix is a separate commit, and each is covered by tests that fail against
main(22 of the 39 new tests do).1.
answer_context_faithfulness_statement_level— score is inverted 🔴answer_context_faithfulness_statement_level.py#L99The variable is named
yes_count, but the ternary yields0when the verdict is "yes" and1otherwise, so it counts unsupported statements.Driving the eval with a scripted grader response:
mainYes. Yes. Yes.No. No. No.Yes. No. Yes.An answer entirely grounded in the retrieved context is scored as a complete hallucination.
The fallback on line 102, taken whenever the grader omits the
Final verdict for each statement in order:summary, is wrong in two further ways:outputis the answer being graded, not the grader's response, so the count is essentially always 0 — and it counts rejections as if they were supported statements. A fully supported answer whose grader reply lacked the summary line scored0.0.Fix: count "yes" verdicts, read the fallback from the grader's response, and clamp to 1.0 (a grader emitting more verdicts than there were statements could otherwise exceed 1).
2.
context_ranking_listwise— NDCG is inverted 🔴context_ranking_listwise.py#L107ndcg(y_true, ranking)expectsy_trueto be relevance grades, butreranked_indicesis a permutation of context indices. The IDCG denominator isdcg(y_true, argsort(y_true)[::-1]), which is maximised when that index list is in descending order — i.e. when the reranker completely reverses the retrieved order.All 24 permutations of 4 contexts, against
sklearn.metrics.ndcg_score:reranked_indicesmain[0, 1, 2, 3][1, 0, 2, 3][2, 1, 3, 0][3, 2, 1, 0]Pearson correlation between the reported score and the correct NDCG: −0.75. A perfect retriever scores 0.55; a retriever that returns its results in exactly the wrong order scores 1.0.
Fix: convert
reranked_indicesinto relevance grades first — the context the reranker placed first gets the highest grade — then measure how well the retrieved order0..n-1agrees with them. Perfect retrieval now scores 1.0, and the regression test asserts that over every permutation of four contexts the score is uniquely maximised by the identity ranking and minimised by the reversed one.Three further bugs on the same path:
listwise_rerankingcannot parse its own prompt's reply format. The prompt labels passagesPassage1..PassageN, but parsing is[int(num) for num in s.split(",") if num.isdigit()]. A reply naming the passages (Passage3, Passage1, ...) parses to[], andcontexts[offset:offset + window_size] = []then silently deletes that window of contexts. A reply of bare numbers is 1-based but is used as 0-based indices, producing an off-by-one ranking and anIndexErrorat the window boundary. Now every integer in the reply is extracted, shifted to 0-based, bounds-checked and de-duplicated, with dropped passages appended — the result is always a permutation.n_contexts_to_rank=1hangs forever. The factory validatesn_contexts_to_rank >= 1, butwindow_step = n_contexts_to_rank // 2is then0, sooffset -= window_stepnever terminates. Confirmed with a 10s alarm: no progress, and no API call is made inside the loop, so it spins on CPU indefinitely. Nowmax(1, n_contexts_to_rank // 2).dcg()overflows int64.gains = 2**rel - 1on an integer array wraps for any relevance grade above 62 —dcg([70], [0])returns-1.0, a negative gain. This is reachable now that grades scale with the number of retrieved contexts (a top-70 retrieval is ordinary).ndcg()also returned NaN rather than 0.0 when nothing was relevant.3.
percent_target_supported_by_context—eval()on model output 🔐percent_target_supported_by_context.py#L84The grader's reply is passed to
eval(), so whatever the model emits is executed as Python. A reply ofpasses the regex and writes the file before any score is computed. Reachable by prompt injection through retrieved documents, which are attacker-controlled in most RAG deployments.
The regex is not a guard at all: it is applied to
classification.replace("\n", "")whileeval()receives the unmodified string, so the two never see the same text. That mismatch also breaks the ordinary case — a grader that pretty-prints its JSON or wraps it in a```jsonfence raisesSyntaxErrorstraight out ofeval().Fix: parse the matched span with
json.loads(via the existingsafe_json_loads) andre.DOTALLso multi-line replies match. A missingAttributedkey no longer raisesAttributeErroronNone.Unparseable replies now return
Noneinstead of0.0.0.0was indistinguishable from "the context supports none of the target" and was logged as a real score;Noneskips the eval, which the return type already allowed. Happy to revert that part if you'd rather keep 0.0.4.
context_ranking_pointwise— one bad response makes the score NaNcontext_ranking_pointwise.py#L80A verification that is not valid JSON, or that omits
verdict, becomesnp.nan, which then poisons both the numerator and the denominator:The NaN is logged as the score and silently destroys any mean computed over the experiment.
Fix: treat an unusable verification as "not relevant", matching RAGAS. numpy is no longer needed, so the import guard and its
Raises:docstring entry are dropped.5.
balanced_acc— fractional scores truncated to zerobalanced_acc.py#L14int()truncates toward zero, so a class whose scores were all0.9is reported as having 0.0 recall. Only an exact1.0counts. Now thresholded at>= 0.5.This is the one judgement call in the PR — if you'd prefer
balanced_accto reject non-binary scores outright, say so and I'll change it.Reproducing
Every case above is covered by
tests/test_evals.py, which drives the eval functions with scripted grader responses viamonkeypatch— no API calls, no keys, runs in ~0.3s.(The
n_contexts_to_rank=1test hangs onmainrather than failing, so deselect it when checking against the old code.)The listwise ranking tests are written as properties rather than fixed numbers — the identity permutation must be the unique maximum and the reversed one the unique minimum — so they catch an inversion regardless of the exact gain formula.
Related Issue
Type of Change
Marked breaking because scores change — that is the point of the PR. Anything scored with the two inverted metrics needs re-running; historical values for them are not comparable to new ones.
Checklist
CODE_OF_CONDUCT.mddocument.CONTRIBUTING.mdguide.make codestyle.